Allow serial number 0 for root CA certificates by jackctj117 · Pull Request #9567 · wolfSSL/wolfssl

jackctj117 · 2025-12-19T22:32:58Z

This pull request updates the logic for validating X.509 certificate serial numbers in wolfcrypt/src/asn.c. The main change is to improve compliance with RFC 5280 while allowing for real-world exceptions involving root CAs. The previous strict check for zero serial numbers is now more nuanced, permitting serial number 0 for self-signed CA certificates but still rejecting it for other certificates.

Certificate serial number validation improvements:

Moved and updated the check for serial numbers of 0 to allow self-signed CA (root) certificates to have a serial number of 0, while still treating serial 0 as an error for all other certificates. This better aligns with RFC 5280 and real-world trust store practices. [1] [2]
Removed the previous check that always treated a serial number of 0 as an error, regardless of certificate type.

Testing

Tested with certificates generated using OpenSSL to verify all scenarios:

Root CA with serial 0 loads successfully (previously failed, now passes)
End-entity cert with serial 0 is correctly rejected
Normal end-entity cert signed by root CA with serial 0 verifies successfully
Self-signed non-CA cert with serial 0 is correctly rejected

kareem-wolfssl

Changes look good.

Can you add some test cases with a CA and leaf cert with a serial of 0?

jackctj117 · 2025-12-23T17:44:25Z

@kareem-wolfssl it looks like the failures are looking for an expected failure due to a self-signed CA certificate with serial number 0.

kareem-wolfssl · 2025-12-23T18:00:32Z

@kareem-wolfssl it looks like the failures are looking for an expected failure due to a self-signed CA certificate with serial number 0.

Yes, you will need to update the failing test test_MakeCertWith0Ser since we no longer expect a root CA with serial 0 to fail.

dgarske · 2025-12-26T15:54:31Z

@jackctj117 looks like some valid unit test failures you will need to look into. All related to --enable-asn=original.

FAILURES:
   390: test_SerialNumber0_RootCA

jackctj117 · 2026-01-14T23:19:18Z

Jenkins retest this please. History lost.

jackctj117 · 2026-01-16T18:32:24Z

Jenkins retest this please. History lost.

jackctj117 · 2026-01-17T00:12:48Z

Jenkins retest this please

jackctj117 · 2026-01-20T21:07:22Z

Jenkins retest this please

jackctj117 · 2026-01-21T16:19:17Z

Jenkins retest this

jackctj117 · 2026-01-21T17:01:02Z

Jenkins retest this please

tests/api/test_asn.c

kareem-wolfssl · 2026-01-30T22:50:38Z

Jenkins retest this please.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

wolfcrypt/src/asn.c:23904

The log message for cert->serialSz == 0 is confusing: it suggests a certificate might legitimately have “no serial number”, but X.509 certificates always have a serial number and RFC 5280 requires it to be present. Consider rewording to clearly state that an empty/zero-length serial is invalid, and (if appropriate in this file) emit the verbose error macro consistently with other parse failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wolfcrypt/src/asn.c

tests/api/test_asn.c

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wolfcrypt/src/asn.c

tests/api/test_asn.c

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

wolfcrypt/src/asn.c:1

The conditional logic has an unbalanced parenthesis structure. The closing parenthesis on line 25812 closes the inner if condition that starts on line 25807, but the logic flow is confusing. The condition should be restructured for clarity: the exception (root CA check) should be more clearly separated from the CSR check. Consider rewriting as: if (!cert->isCSR && !((type == CA_TYPE || type == TRUSTED_PEER_TYPE) && cert->isCA && cert->selfSigned))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… test

…est assertions

…e conditions

…riable errors

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

wolfcrypt/src/asn.c:1

This introduces an early return from ParseCertRelative. If the rest of the function relies on a single-exit pattern for consistent cleanup/logging (common in wolfSSL code), this return can bypass it. Prefer setting ret = ASN_PARSE_E and flowing through the function’s existing error/exit handling (e.g., via the established goto exit; pattern if present).
wolfcrypt/src/asn.c:1
The updated log message format is inconsistent with nearby wolfSSL messages (it adds an Error: prefix and includes parenthetical text). Consider aligning with existing WOLFSSL_MSG style (e.g., no Error: prefix, keep it short, and/or mention the relevant compile-time knob only where actionable) to keep logs consistent and easier to grep.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T21:01:10Z

tests/api/test_asn.c

+    ExpectNotNull(cm = wolfSSL_CertManagerNew());
+    ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm, rootNormalFile, NULL),
+                WOLFSSL_SUCCESS);
+


If the #if (!defined(NO_WOLFSSL_CLIENT) ... ) block is compiled out, cm allocated in Test 2 is never freed (the only free happens inside that conditional block). Free cm unconditionally after Test 2 (or restructure to scope the allocation to the block) to avoid a configuration-dependent leak in the test.

Suggested change

if (cm != NULL) {

wolfSSL_CertManagerFree(cm);

cm = NULL;

}

Copilot · 2026-03-04T21:01:11Z

tests/api/test_asn.c

+    ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),
+                WOLFSSL_SUCCESS);


This assertion only checks LoadCA fails, but LoadCA can fail for reasons unrelated to the new serial-0 logic (e.g., the certificate is CA:FALSE). To specifically cover the behavior added in ParseCertRelative (reject serial 0 for non-root certificates), assert an error that is attributable to serial-0 rejection (e.g., check for ASN_PARSE_E on a code path that parses it as a non-CA/non-trust-anchor certificate), or otherwise tighten the expectation so the test fails if serial 0 is accidentally allowed for non-root certificates.

Suggested change

ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),

WOLFSSL_SUCCESS);

ExpectIntEQ(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),

WC_NO_ERR_TRACE(ASN_PARSE_E));

douzzer · 2026-03-05T05:25:38Z

retest this please

jackctj117 self-assigned this Dec 19, 2025

kareem-wolfssl reviewed Dec 19, 2025

View reviewed changes

kareem-wolfssl mentioned this pull request Dec 19, 2025

[Bug]: WolfSSL accepts a certificate whose serial number is zero #8615

Open

jackctj117 requested a review from kareem-wolfssl December 23, 2025 17:43

tmael mentioned this pull request Dec 23, 2025

[Bug]: curl: (35) SSL_connect failed with error -140: ASN parsing error, invalid input #7844

Closed

kareem-wolfssl requested changes Jan 22, 2026

View reviewed changes

tests/api/test_asn.c Outdated Show resolved Hide resolved

tests/api/test_asn.c Outdated Show resolved Hide resolved

kareem-wolfssl requested a review from wolfSSL-Bot January 22, 2026 21:47

jackctj117 requested a review from kareem-wolfssl January 30, 2026 22:38

jackctj117 force-pushed the serial-0 branch from 82c6f7e to af2579c Compare February 3, 2026 21:45

jackctj117 requested a review from Copilot February 5, 2026 19:12

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Copilot started reviewing on behalf of jackctj117 February 5, 2026 20:00 View session

jackctj117 requested a review from Copilot February 5, 2026 20:20

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Copilot started reviewing on behalf of jackctj117 February 5, 2026 20:35 View session

jackctj117 requested a review from Copilot February 6, 2026 18:09

Copilot started reviewing on behalf of jackctj117 February 6, 2026 18:11 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

wolfcrypt/src/asn.c Outdated Show resolved Hide resolved

tests/api/test_asn.c Show resolved Hide resolved

jackctj117 requested a review from Copilot February 11, 2026 18:49

Copilot started reviewing on behalf of jackctj117 February 11, 2026 18:50 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

wolfcrypt/src/asn.c Outdated Show resolved Hide resolved

tests/api/test_asn.c Outdated Show resolved Hide resolved

jackctj117 requested a review from Copilot February 18, 2026 21:39

Copilot AI reviewed Feb 18, 2026

View reviewed changes

Copilot started reviewing on behalf of jackctj117 February 18, 2026 21:44 View session

jackctj117 requested review from kareem-wolfssl and removed request for kareem-wolfssl and wolfSSL-Bot February 19, 2026 22:49

jackctj117 assigned wolfSSL-Bot Feb 24, 2026

jackctj117 requested a review from SparkiDev February 28, 2026 00:03

SparkiDev previously approved these changes Mar 2, 2026

View reviewed changes

jackctj117 added 14 commits March 4, 2026 13:18

Allow serial number 0 for root CA certificates

49d9b9d

Added serial 0 tests to unit tests

b187272

Fix for old tests

4fd35da

Add serial 0 validation to non-template code path

2d82aee

Add serial 0 validation to original parser, fix CSR handling

d538365

Fix test_MakeCertWith0Ser: remove keyCertSign from non-CA certificate…

14ecd15

… test

Add generated test certificates for serial 0 testing

5b59abc

Added new certs to include.am

8678c6d

Fixes for tests

4df0f8d

Copilot fixes

69b25a6

Fix CI failures: serial-0 error flow, PEM guard, and codespell CSR skip

d328582

Fix serial-0 check to allow TRUSTED_PEER_TYPE load path and correct t…

5ed4b27

…est assertions

Guard CertManagerVerify test calls behind client/OPENSSL_EXTRA compil…

bf18d3e

…e conditions

Move ee cert path variables inside client-auth guard to fix unused-va…

8b5bde4

…riable errors

Copilot AI review requested due to automatic review settings March 4, 2026 20:54

jackctj117 dismissed SparkiDev’s stale review via 8b5bde4 March 4, 2026 20:54

jackctj117 force-pushed the serial-0 branch from e269c50 to 8b5bde4 Compare March 4, 2026 20:54

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Copilot started reviewing on behalf of jackctj117 March 5, 2026 01:24 View session

+    if (cm != NULL) {
+        wolfSSL_CertManagerFree(cm);
+        cm = NULL;
+    }

		ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, selfSignedNonCASerial0File, NULL),
		WOLFSSL_SUCCESS);

Conversation

jackctj117 commented Dec 19, 2025

Testing

Uh oh!

kareem-wolfssl left a comment

Choose a reason for hiding this comment

Uh oh!

jackctj117 commented Dec 23, 2025

Uh oh!

kareem-wolfssl commented Dec 23, 2025

Uh oh!

dgarske commented Dec 26, 2025

Uh oh!

jackctj117 commented Jan 14, 2026

Uh oh!

jackctj117 commented Jan 16, 2026

Uh oh!

jackctj117 commented Jan 17, 2026

Uh oh!

jackctj117 commented Jan 20, 2026

Uh oh!

jackctj117 commented Jan 21, 2026

Uh oh!

jackctj117 commented Jan 21, 2026

Uh oh!

Uh oh!

Uh oh!

kareem-wolfssl commented Jan 30, 2026 • edited by jackctj117 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

douzzer commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kareem-wolfssl commented Jan 30, 2026 •

edited by jackctj117

Loading